[gazebo_plugins] Fix for issue #408 - correct the timestamp used by the camera #410

lucasw · 2016-03-22T04:26:27Z

Fix for #408

The last measurement time is the time that gazebo generated the sensor data, so ought to be used.

The other cameras need similar fixes to have the proper timestamps.

There is a test to show that the timestamp is good, but issue #409 has lead me to comment it out for now.

There probably ought to be a separate issue about what updateRate is supposed to be for, and if it isn't useful remove it (from all the sensors that have it) in another PR.

scpeters · 2016-04-25T23:19:45Z

There's a gzserver failure in the travis test:

gzserver: /usr/include/boost/smart_ptr/shared_ptr.hpp:653: typename boost::detail::sp_member_access<T>::type boost::shared_ptr<T>::operator->() const [with T = gazebo::rendering::DepthCamera; typename boost::detail::sp_member_access<T>::type = gazebo::rendering::DepthCamera*]: Assertion `px != 0' failed.

Aborted (core dumped)

lucasw · 2016-04-26T00:07:14Z

Can the travis ci build be triggered for 596b302? That had no code changes to the depth camera, but ran it the depth camera in a test which might indicate some other issue that is independent of my changes. The test should still fail, but because of timestamp issues, not the px assertions.

The test that fails wouldn't actually be enabled when merged (the test set_model_state-test would be re-enabled until #409 is resolved).

scpeters · 2016-04-26T01:11:59Z

I don't have permissions to trigger builds for a specific commit. I'll try making a separate branch...

scpeters · 2016-04-26T19:18:09Z

I pushed 596b302 to my fork ( scpeters@596b302 ) and am awaiting the travis results:

https://travis-ci.org/scpeters/gazebo_ros_pkgs/builds/125919069

scpeters · 2016-04-26T19:38:52Z

It looks like 596b302 also fails. It could be because these buildfarm machines don't have GPU's. @j-rivero mentioned that he would figure out how to disable tests that require a GPU if one is not available.

lucasw · 2016-04-26T19:59:19Z

Yeah it is the same px != 0 failure (as opposed to my test failing because timestamps are wrong). Does it actually need a gpu, or would a software virtual framebuffer work?

I think this test out to be commented out and the set_model_state-test test restored. But can someone other than me try out the changes manually (both running the tests and loading the depth, multi, and regular camera world files in a regular session and viewing published topics).

j-rivero · 2016-04-26T21:57:22Z

Yeah it is the same px != 0 failure (as opposed to my test failing because timestamps are wrong).

I was able to reproduce the error when there is no valid DISPLAY available

Does it actually need a gpu, or would a software virtual framebuffer work?

I think that a virtual framebuffer would be enough, @iche033 what do you think?

I think this test out to be commented out and the set_model_state-test test restored.

I'm interested into enabling all the possible tests. To run the test one by one we can use the -j1 argument. Maybe we can this option:

-- a/gazebo_plugins/CMakeLists.txt
+++ b/gazebo_plugins/CMakeLists.txt
@@ -1,6 +1,8 @@
 cmake_minimum_required(VERSION 2.8.3)
 project(gazebo_plugins)

+option(ENABLE_DISPLAY_TESTS "Enable the building of tests that requires a display" ON)
+
 find_package(catkin REQUIRED COMPONENTS 
   message_generation 
   gazebo_msgs 
@@ -339,22 +341,23 @@ install(DIRECTORY test
 # Tests
 if (CATKIN_ENABLE_TESTING)
   find_package(rostest REQUIRED)
-  # add_rostest_gtest(set_model_state-test
-  #                   test/set_model_state_test/set_model_state_test.test 
-  #                   test/set_model_state_test/set_model_state_test.cpp)
-  # target_link_libraries(set_model_state-test ${catkin_LIBRARIES})
-
-  # Can't run these and the above test together
-  add_rostest_gtest(depth_camera-test
-                    test/camera/depth_camera.test
-                    test/camera/depth_camera.cpp)
-  target_link_libraries(depth_camera-test ${catkin_LIBRARIES})
-  # add_rostest_gtest(multicamera-test
-  #                   test/camera/multicamera.test
-  #                   test/camera/multicamera.cpp)
-  # target_link_libraries(multicamera-test ${catkin_LIBRARIES})
-  # add_rostest_gtest(camera-test
-  #                   test/camera/camera.test
-  #                   test/camera/camera.cpp)
-  # target_link_libraries(camera-test ${catkin_LIBRARIES})
+  add_rostest_gtest(set_model_state-test
+                    test/set_model_state_test/set_model_state_test.test 
+                    test/set_model_state_test/set_model_state_test.cpp)
+  target_link_libraries(set_model_state-test ${catkin_LIBRARIES})
+
+  if (ENABLE_DISPLAY_TESTS)
+    add_rostest_gtest(depth_camera-test
+                     test/camera/depth_camera.test
+                     test/camera/depth_camera.cpp)
+    target_link_libraries(depth_camera-test ${catkin_LIBRARIES})
+    add_rostest_gtest(multicamera-test
+                      test/camera/multicamera.test
+                      test/camera/multicamera.cpp)
+    target_link_libraries(multicamera-test ${catkin_LIBRARIES})
+    add_rostest_gtest(camera-test
+                     test/camera/camera.test
+                     test/camera/camera.cpp)
+    target_link_libraries(camera-test ${catkin_LIBRARIES})
+  endif()
 endif()

But can someone other than me try out the changes manually (both running the tests and loading the depth, multi, and regular camera world files in a regular session and viewing published topics).

Test are failing on my local system:

[ROSUNIT] Outputting test results to /tmp/tmp.tVYKeyj2OF/build/gazebo_plugins/test_results/gazebo_plugins/rostest-test_camera_depth_camera.xml
[ INFO] [1461707320.297311178]: Finished loading Gazebo ROS API Plugin.
[ INFO] [1461707320.297679763]: waitForService: Service [/gazebo/set_physics_properties] has not been advertised, waiting...
Error [SDF.cc:789] Missing element description for [pointCloudTopicName]
Error [SDF.cc:789] Missing element description for [depthImageCameraInfoTopicName]
Error [SDF.cc:789] Missing element description for [pointCloudCutoff]
[ INFO] [1461707321.006708696]: Camera Plugin (robotNamespace = ), Info: the 'robotNamespace' param did not exit
[ INFO] [1461707321.032785687, 0.023000000]: waitForService: Service [/gazebo/set_physics_properties] is now available.
[ INFO] [1461707321.033451535, 0.024000000]: Camera Plugin (ns = )  <tf_prefix_>, set to ""
[ INFO] [1461707321.086888269, 0.074000000]: Physics dynamic reconfigure ready.
testdepth_camera ... ok

[ROSTEST]-----------------------------------------------------------------------

[gazebo_plugins.rosunit-depth_camera/cameraSubscribeTest][FAILURE]--------------
/tmp/tmp.tVYKeyj2OF/src/gazebo_ros_pkgs/gazebo_plugins/test/camera/depth_camera.cpp:90
Expected: (time_diff) < (0.5), actual: 0.523 vs 0.5
--------------------------------------------------------------------------------


[gazebo_plugins.rosunit-depth_camera/cameraSubscribeTest][FAILURE]--------------
/tmp/tmp.tVYKeyj2OF/src/gazebo_ros_pkgs/gazebo_plugins/test/camera/depth_camera.cpp:94
Expected: (time_diff) < (0.5), actual: 0.523 vs 0.5
--------------------------------------------------------------------------------


[gazebo_plugins.rosunit-depth_camera/cameraSubscribeTest][FAILURE]--------------
/tmp/tmp.tVYKeyj2OF/src/gazebo_ros_pkgs/gazebo_plugins/test/camera/depth_camera.cpp:98
Expected: (time_diff) < (0.5), actual: 0.523 vs 0.5
--------------------------------------------------------------------------------

lucasw · 2016-04-26T23:05:18Z

Tests are failing on my local system:

@j-rivero Are you on the latest c033556 ?

j-rivero · 2016-04-26T23:50:35Z

@j-rivero Are you on the latest c033556 ?

unless I have something very broken, I see that commit in the git log on my branch.

lucasw · 2016-04-27T00:26:02Z

Expected: (time_diff) < (0.5), actual: 0.523 vs 0.5

So if the new code wasn't working at all the time difference would be more than 2 seconds (the update period of the sensor in the test world file)- and looking now my computer is taking about 0.3 seconds. I just chose 0.5 thinking it was generous at the time, but it could be increased to say 1.0. If any more is required I'd like to decrease the sensor update rate to get a more generous margin on both sides.

scpeters · 2016-04-27T00:57:33Z

Sounds fine to me to relax the tolerances

j-rivero · 2016-05-25T01:32:01Z

Thanks for the fixes. Now I only have one error if I run the test locally on my Intel laptop:

[ INFO] [1464139734.627722818]: waitForService: Service [/gazebo/set_physics_properties] has not been advertised, waiting...
Error [parser.cc:292] Error parsing XML in file [/home/jrivero/code/ros/ws/pr_410/src/gazebo_ros_pkgs/gazebo_plugins/test/camera/depth_camera.world]: Error reading end tag.
gzserver: /usr/include/boost/thread/pthread/recursive_mutex.hpp:101: boost::recursive_mutex::~recursive_mutex(): Assertion `!pthread_mutex_destroy(&m)' failed.
Aborted (core dumped)
testdepth_camera ... ERROR!
ERROR: max time [30.0s] allotted for test [depth_camera] of type [gazebo_plugins/depth_camera-test]

To able to keep our CI headless working, can we please try to merge my previous proposed patch (I've changed the default not to run DISPLAY tests):

-- a/gazebo_plugins/CMakeLists.txt
+++ b/gazebo_plugins/CMakeLists.txt
@@ -1,6 +1,8 @@
 cmake_minimum_required(VERSION 2.8.3)
 project(gazebo_plugins)

+option(ENABLE_DISPLAY_TESTS "Enable the building of tests that requires a display" OFF)
+
 find_package(catkin REQUIRED COMPONENTS 
   message_generation 
   gazebo_msgs 
@@ -339,22 +341,23 @@ install(DIRECTORY test
 # Tests
 if (CATKIN_ENABLE_TESTING)
   find_package(rostest REQUIRED)
-  # add_rostest_gtest(set_model_state-test
-  #                   test/set_model_state_test/set_model_state_test.test 
-  #                   test/set_model_state_test/set_model_state_test.cpp)
-  # target_link_libraries(set_model_state-test ${catkin_LIBRARIES})
-
-  # Can't run these and the above test together
-  add_rostest_gtest(depth_camera-test
-                    test/camera/depth_camera.test
-                    test/camera/depth_camera.cpp)
-  target_link_libraries(depth_camera-test ${catkin_LIBRARIES})
-  # add_rostest_gtest(multicamera-test
-  #                   test/camera/multicamera.test
-  #                   test/camera/multicamera.cpp)
-  # target_link_libraries(multicamera-test ${catkin_LIBRARIES})
-  # add_rostest_gtest(camera-test
-  #                   test/camera/camera.test
-  #                   test/camera/camera.cpp)
-  # target_link_libraries(camera-test ${catkin_LIBRARIES})
+  add_rostest_gtest(set_model_state-test
+                    test/set_model_state_test/set_model_state_test.test 
+                    test/set_model_state_test/set_model_state_test.cpp)
+  target_link_libraries(set_model_state-test ${catkin_LIBRARIES})
+
+  if (ENABLE_DISPLAY_TESTS)
+    add_rostest_gtest(depth_camera-test
+                     test/camera/depth_camera.test
+                     test/camera/depth_camera.cpp)
+    target_link_libraries(depth_camera-test ${catkin_LIBRARIES})
+    add_rostest_gtest(multicamera-test
+                      test/camera/multicamera.test
+                      test/camera/multicamera.cpp)
+    target_link_libraries(multicamera-test ${catkin_LIBRARIES})
+    add_rostest_gtest(camera-test
+                     test/camera/camera.test
+                     test/camera/camera.cpp)
+    target_link_libraries(camera-test ${catkin_LIBRARIES})
+  endif()
 endif()

`

…me that gazebo generated the sensor data, so ought to be used. updateRate doesn't seem that useful. The other cameras need similar fixes to have the proper timestamps.

…amps are currently outdated, will fix them similar to how the regular camera was fixed.

…an outdated, also reuse the same update code

…ght timestamp ros-simulation#408- appears to be working (though only looking at horizon) but getting these sdf errors: Error [SDF.cc:789] Missing element description for [pointCloudTopicName] Error [SDF.cc:789] Missing element description for [depthImageCameraInfoTopicName] Error [SDF.cc:789] Missing element description for [pointCloudCutoff]

…t make it pass

…g 0.6 seconds to receive the messages (still well less than 2.0 seconds). Also all the tests can be run with run_tests_gazebo_plugins_rostest but only with the -j1 flag ros-simulation#409

lucasw · 2016-10-18T20:39:22Z

I've made the tests disabled by default unless ENABLE_DISPLAY_TESTS is on, and rebased off the latest jade-devel.

codebot · 2017-01-26T22:52:37Z

I haven't tried the tests myself, but we just independently ran into the same bug and chased down the same underlying fix (using LastMeasurementTime instead of LastUpdateTime). It indeed fixes the timestamp problems on the ROS side.

iche033 · 2017-02-03T19:39:45Z

see this related issue in the gazebo repo. I made similar changes to the openni_kinect plugin and the timestamp issue is fixed for me.

iche033 · 2017-02-06T23:25:09Z

gazebo_plugins/test/camera/depth_camera.world

+          <depthImageTopicName>depth/image_raw</depthImageTopicName>
+          <!-- neither camera info is getting published, frame_id is empty 
+            in points and both image headers -->
+          <depthImageTopicCameraInfoName>depth/camera_info</depthImageCameraInfoTopicName>


I ran into an xml parser error that turned out to be due to this line. There is a mismatch between open and end tags

iche033 · 2017-02-06T23:27:14Z

had to make a minor fix for the depth camera test to run. After that, all tests passed for me

diff --git a/gazebo_plugins/test/camera/depth_camera.world b/gazebo_plugins/test/camera/depth_camera.world
index 978a0a4..73132f9 100644
--- a/gazebo_plugins/test/camera/depth_camera.world
+++ b/gazebo_plugins/test/camera/depth_camera.world
@@ -124,7 +124,7 @@
           <depthImageTopicName>depth/image_raw</depthImageTopicName>
           <!-- neither camera info is getting published, frame_id is empty 
             in points and both image headers -->
-          <depthImageTopicCameraInfoName>depth/camera_info</depthImageCameraInfoTopicName>
+          <depthImageCameraInfoTopicName>depth/camera_info</depthImageCameraInfoTopicName>
           <pointCloudTopicName>points</pointCloudTopicName>
           <frameName>camera_link</frameName>
           <!-- TODO(lucasw) is this used by depth camera at all? -->

j-rivero · 2017-02-10T16:50:35Z

had to make a minor fix for the depth camera test to run. After that, all tests passed for me

Thanks Ian for the testing. @lucasw if you please could apply the patch that Ian mentioned, it would be great so I can propagate the change to the different branches with the fix included.

j-rivero · 2017-02-11T18:22:00Z

Thanks Ian for the testing. @lucasw if you please could apply the patch that Ian mentioned, it would be great so I can propagate the change to the different branches with the fix included.

Don't worry, let's merge this PR and continue with the pending changes in pr #538

This PR builds on top of pull request #410 and applies the timestamp fix to kinect_openni and prosilica sensors

This PR builds on top of pull request ros-simulation#410 and applies the timestamp fix to kinect_openni and prosilica sensors

lucasw changed the title ~~[gazebo_pluginsFix for issue #408 - correct the timestamp used by the camera~~ [gazebo_plugins] Fix for issue #408 - correct the timestamp used by the camera Mar 22, 2016

scpeters added this to the untargeted milestone Apr 25, 2016

scpeters assigned j-rivero Apr 25, 2016

j-rivero mentioned this pull request Apr 26, 2016

Option to disable tests that requires a valid display or GPU #438

Open

lucasw mentioned this pull request Oct 7, 2016

[gazebo_plugins] camera sensor timestamp is one update old #408

Closed

lucasw added 9 commits October 18, 2016 09:41

Fix for issue ros-simulation#408. The last measurement time is the ti…

d00bd47

…me that gazebo generated the sensor data, so ought to be used. updateRate doesn't seem that useful. The other cameras need similar fixes to have the proper timestamps.

ros-simulation#408 Making a test for multicamra that shows the timest…

3fb2fea

…amps are currently outdated, will fix them similar to how the regular camera was fixed.

ros-simulation#408 Make the multi camera timestamps current rather th…

34b45d0

…an outdated, also reuse the same update code

Disabling this test because of ros-simulation#409

546f285

ros-simulation#408 Created test for depth camera, which fails, so nex…

58d47ae

…t make it pass

ros-simulation#408 also test points publication

b1dec2f

ros-simulation#408 fix timestamps for depth image, test passes now

2be9a20

ros-simulation#408 Increasing max time because some systems are takin…

9b56df9

…g 0.6 seconds to receive the messages (still well less than 2.0 seconds). Also all the tests can be run with run_tests_gazebo_plugins_rostest but only with the -j1 flag ros-simulation#409

lucasw force-pushed the camera_timestamp_408 branch from fdb1895 to 9b56df9 Compare October 18, 2016 17:08

Adding @j-rivero default disabling of tests that require a display

3dc0df1

iche033 reviewed Feb 6, 2017

View reviewed changes

iche033 mentioned this pull request Feb 10, 2017

Fix timestamp issues for rendering sensors #538

Merged

j-rivero merged commit ab36b5d into ros-simulation:jade-devel Feb 13, 2017

This was referenced Feb 13, 2017

Correct the timestamp used by the camera (kinetic-devel) #540

Merged

Fix timestamp issues for rendering sensors (indigo-devel) #544

Closed

j-rivero added a commit that referenced this pull request Feb 16, 2017

Fix timestamp issues for rendering sensors (kinetic-devel)

7b84169

This PR builds on top of pull request #410 and applies the timestamp fix to kinect_openni and prosilica sensors

j-rivero mentioned this pull request Feb 16, 2017

Fix timestamp issues for rendering sensors (kinetic-devel) #545

Merged

iche033 mentioned this pull request Feb 22, 2017

The time stamp in the header of the message camera/depth/image_raw is occasionally set to zero. #548

Closed

j-rivero mentioned this pull request Feb 27, 2017

Fix timestamp issues for rendering sensors (indigo-devel) #551

Merged

This was referenced Apr 19, 2020

Significant sensor data timestamp accuracy regression from Gazebo2 to Gazebo4 gazebosim/gazebo-classic#1748

Closed

Gazebo is setting "sensor's last update time" to 0 gazebosim/gazebo-classic#2048

Open

cohen39 pushed a commit to cohen39/gazebo_ros_pkgs that referenced this pull request Nov 15, 2021

Fix timestamp issues for rendering sensors (kinetic-devel)

d33c480

This PR builds on top of pull request ros-simulation#410 and applies the timestamp fix to kinect_openni and prosilica sensors

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[gazebo_plugins] Fix for issue #408 - correct the timestamp used by the camera #410

[gazebo_plugins] Fix for issue #408 - correct the timestamp used by the camera #410

lucasw commented Mar 22, 2016

scpeters commented Apr 25, 2016

lucasw commented Apr 26, 2016

scpeters commented Apr 26, 2016

scpeters commented Apr 26, 2016

scpeters commented Apr 26, 2016

lucasw commented Apr 26, 2016

j-rivero commented Apr 26, 2016 •

edited

Loading

lucasw commented Apr 26, 2016

j-rivero commented Apr 26, 2016

lucasw commented Apr 27, 2016 •

edited

Loading

scpeters commented Apr 27, 2016

j-rivero commented May 25, 2016

lucasw commented Oct 18, 2016

codebot commented Jan 26, 2017

iche033 commented Feb 3, 2017

iche033 Feb 6, 2017

iche033 commented Feb 6, 2017

j-rivero commented Feb 10, 2017

j-rivero commented Feb 11, 2017

[gazebo_plugins] Fix for issue #408 - correct the timestamp used by the camera #410

[gazebo_plugins] Fix for issue #408 - correct the timestamp used by the camera #410

Conversation

lucasw commented Mar 22, 2016

scpeters commented Apr 25, 2016

lucasw commented Apr 26, 2016

scpeters commented Apr 26, 2016

scpeters commented Apr 26, 2016

scpeters commented Apr 26, 2016

lucasw commented Apr 26, 2016

j-rivero commented Apr 26, 2016 • edited Loading

lucasw commented Apr 26, 2016

j-rivero commented Apr 26, 2016

lucasw commented Apr 27, 2016 • edited Loading

scpeters commented Apr 27, 2016

j-rivero commented May 25, 2016

lucasw commented Oct 18, 2016

codebot commented Jan 26, 2017

iche033 commented Feb 3, 2017

iche033 Feb 6, 2017

Choose a reason for hiding this comment

iche033 commented Feb 6, 2017

j-rivero commented Feb 10, 2017

j-rivero commented Feb 11, 2017

j-rivero commented Apr 26, 2016 •

edited

Loading

lucasw commented Apr 27, 2016 •

edited

Loading